feat(web): highlight resolved mentions - #137
Conversation
clawsweeper
Bot
added
proof: 📸 screenshot
|
Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 8:47 AM ET / 12:47 UTC. ClawSweeper reviewWhat this changesAdds post-Markdown highlighting for known workspace Merge readinessThis PR remains necessary because current Priority: P2 Review scores
Verification
How this fits togetherClickClack renders stored conversation text as Markdown in its main web application and embedded channel/thread views. The proposed decorator receives workspace member targets after rendering and annotates matching visible handles without altering message storage or delivery. flowchart LR
A[Workspace member records] --> B[Mention target list]
C[Rendered Markdown text] --> D[Mention decorator]
B --> D
D --> E[Main channel and thread views]
D --> F[Embedded channel and thread views]
D --> G[Agent commentary and tool preambles]
Decision needed
Why: The implementation selects a broader visible-text policy than the existing capped local people list, so maintainer intent is needed in addition to mechanical conflict resolution. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Rebase the focused decorator onto current Do we have a high-confidence way to reproduce the issue? Yes for the current merge blocker: a source-level three-way merge against current Is this the best way to solve the issue? No, not as submitted: the decorator is focused, but the branch must be rebased and the embedded bundle regenerated; maintainers must also confirm its workspace-wide targeting policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 72931b10f083. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (21 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
removed
the
proof: 📸 screenshot
clawsweeper
Bot
added
proof: 📸 screenshot
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1acfbb95ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| do { | ||
| const page = await listWorkspaceMembersPage({ | ||
| workspaceID, | ||
| cursor, | ||
| limit: 100, | ||
| }); | ||
| members.push(...page.members.map((member) => member.user)); | ||
| cursor = page.has_more ? page.next_cursor : undefined; | ||
| } while (cursor); |
There was a problem hiding this comment.
When a workspace has more than 100 members, this loop issues one request per page serially, and applyRoute awaits the entire loadWorkspaceMembers call before resolving the route target and loading its messages. Opening or switching a large workspace can therefore leave the conversation blocked behind dozens or hundreds of member-list requests; load these targets in the background or resolve only the handles needed for visible messages.
Useful? React with 👍 / 👎.
| if (serial !== workspaceMembersLoadSerial || workspaceID !== selectedWorkspaceID) return; | ||
| workspaceMemberUsers = members; |
There was a problem hiding this comment.
This assignment creates a snapshot that is only refreshed when the workspace changes. The existing bot.deleted and bot.membership_removed realtime handlers reload direct conversations, moderation data, and commands but never this list, so a removed bot's old handle remains highlighted as resolvable—and a newly added or replacement bot is absent—until the user reloads or switches workspaces.
Useful? React with 👍 / 👎.
| return { | ||
| update(nextPeople: User[] = []) { | ||
| targets = mentionTargets(nextPeople); | ||
| renderMentionHighlights(node, targets); | ||
| }, |
There was a problem hiding this comment.
The action updates only when mentionPeople changes, not when the element's {@html markdown(message.body)} content changes. For example, a realtime edit of a reply in the currently open thread replaces the reply HTML while the workspace people array remains unchanged, leaving mentions in the edited body unhighlighted until some unrelated people-list update or remount; observe child mutations or pass the body/version into the action.
Useful? React with 👍 / 👎.
| function mentionIsInsideURL(text: string, start: number): boolean { | ||
| const tokenStart = | ||
| Math.max(text.lastIndexOf(" ", start - 1), text.lastIndexOf("\n", start - 1)) + 1; | ||
| const prefix = text.slice(tokenStart, start); | ||
| return /^(?:www\.[^\s/]+|[A-Za-z][A-Za-z0-9+.-]*:\/\/[^\s/]+)\//u.test(prefix); |
There was a problem hiding this comment.
The URL check only recognizes prefixes beginning with www. or a URI scheme. Plain URL-like paths such as /users/@alice and docs/path/@alice?tab=1 therefore highlight @alice, despite the feature's intended exclusion of URL paths; detect slash-delimited path tokens as well as fully qualified URLs.
Useful? React with 👍 / 👎.
| <MessageList | ||
| {messages} | ||
| selectedChannel={channel} | ||
| {mentionPeople} |
There was a problem hiding this comment.
The embedded channel passes a mentionPeople list derived only from the authors in the currently loaded message window plus the current user, capped at 24 entries; EmbedThreadView similarly uses only conversation members and loaded authors. Consequently, a valid workspace mention remains plain whenever its target has not authored one of those loaded messages, so embedded views do not consistently provide the resolved-mention treatment added here; fetch or otherwise resolve workspace targets for embeds as well.
Useful? React with 👍 / 👎.
| {:else} | ||
| <QuoteBlock {message} onJump={onJumpToQuote} /> | ||
| <div class="markdown" use:enhanceMarkdown>{@html markdown(message.body)}</div> | ||
| <div class="markdown" use:enhanceMarkdown use:enhanceMentions={mentionPeople}>{@html markdown(message.body)}</div> |
There was a problem hiding this comment.
The new action is attached only in the ordinary-message branch, while coalesced agent_commentary rows take the earlier PreambleBlock branch and render their commentary Markdown with only enhanceMarkdown. A bot message such as Handing this to @alice therefore remains unhighlighted when it is displayed as a preamble, which omits the semantic treatment from the agent-handoff content this feature is intended to make scannable; pass the mention targets into PreambleBlock and enhance its commentary bodies too.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
rating: 🦐 gold shrimp
clawsweeper
Bot
added
the
rating: 🦪 silver shellfish
clawsweeper
Bot
added
proof: sufficient
1 participant
What changed
@handlementions in normal channel and thread messages.Why
Rendered ClickClack messages showed resolvable workspace handles as ordinary text, making agent handoffs difficult to scan. This is a presentation-only decorator applied after Markdown rendering; storage, delivery, and Markdown parsing are unchanged.
Review findings addressed
The six Codex connector findings are addressed in commit
b5be20da03e560784c42fd695819d5546b197a34:Browser proof
After the fixes, Chromium loaded the branch's Vite-served proof page using the real
enhanceMentionsaction and one redacted workspace target (member-29). The live DOM snapshot showed all three rendering surfaces decorated:This directly verifies the shared decorator wiring for ordinary, embedded, and agent-commentary content. The existing boundary tests additionally verify that unknown handles, emails, URL paths, links, and code remain untouched.
Validation
node --test src/lib/*.test.ts— 20 passed.pnpm typecheck— passed on current head.pnpm fmt:ts:check— passed on current head; the prior CI failure was a formatting issue inmention-people.test.ts, fixed in65c9638.pnpm --filter @clickclack/web build— passed.<mark>.Related to #116.
AI-assisted implementation and review.